Update model_to() and load_torch_model() methods in ModelABC #728
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Refactor base code from IOSegmentorConfig to IOConfigABC
- No need for a separate function
- Enable git workflow on this PR
- Add missing variable
- Move `to_baseline` code to ABC
- Move `to_baseline` code to ABC
- Fix incorrect calculations for output_resolutions
- Update `PatchPredictor` to use `ModelIOConfigABC` Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Define `ModelIOConfigABC` as a dataclass Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Remove kwargs. - Define dataclass attributes. Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Add `IOInstanceSegmentorConfig` Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Update yaml for `IOInstanceSegmentorConfig` Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Update yaml for `IOInstanceSegmentorConfig` Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Update `to_baseline` for `IOInstanceSegmentorConfig` Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Update IOInstanceSegmentorConfig in test_nucleus_instance_segmentor.py Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Update cli for IOInstanceSegmentorConfig Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Remove unnecessary test after kwargs removal Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Move ioconfigs to io_config.py Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Fix circular import Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Refactor to reduce code in inheriting class Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Fix deepsource errors Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Fix ioconfig import to avoid circular imports Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Fix missing input arguments Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Improve error catching Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Default configuration is not correct Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Fix error check Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Fix typo Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Fix unnecessary update Signed-off-by: Shan E Ahmed Raza <[email protected]>
for more information, see https://pre-commit.ci
…ueImageAnalytics/tiatoolbox into dev-redefine-patchpredictor
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…ueImageAnalytics/tiatoolbox into dev-redefine-patchpredictor
…-redefine-patchpredictor # Conflicts: # tests/test_utils.py # tiatoolbox/utils/misc.py
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## dev-define-engines-abc #728 +/- ##
===========================================================
- Coverage 99.77% 88.36% -11.42%
===========================================================
Files 63 67 +4
Lines 6784 7656 +872
Branches 1352 1491 +139
===========================================================
- Hits 6769 6765 -4
- Misses 7 874 +867
- Partials 8 17 +9
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Closing in favour of #733 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR tracks the changes as per suggestions from @mostafajahanifar in PR #635
Suggestion 1
To follow the usual convention of moving model first and then using DataParallelism, I would suggest improving this function like below:
This will also avoid unnecessary overhead of DataParallel is there is only one GPU available.
Again, this can be integrated as a method into the ModelABC class. I mean, it should already has
tomethod inherited fromnn.Module. However, if we needtorch.nn.DataParallel, we can replace thattomethod with this one. Then users can call:my_model.to(device)Suggestion 2
why not move this function into the ModelABC class as a method? So, users can load model weights for our models just like they do with normal Pytorch models? is it possible something like below:
my_model.load_weights_from_path(path)ormy_model.load(path)I assume because ModelABC is inheriting from
nn.module, it should already haveload_state_dictmethod.